Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Description: RG-T129 Notification System Bug Fixes
Summary
This PR fixes several bugs in the notification system that prevented notifications from firing correctly and caused runtime errors under certain conditions.
Changes
Bug Fix: Incorrect Event Type in Group Lookup
In
GetGroupForEventAsync, the code block that looks up personnel staffing data was incorrectly matched againstPersonnelStatusChangedinstead ofPersonnelStaffingChanged. This meant group-based notifications for staffing changes would never resolve the correct department group.Bug Fix: Empty BeforeData/CurrentData Causing Notifications to Never Fire
The notification validation logic previously returned
falsewheneverBeforeDataorCurrentDatawas null or empty. Since the UI's "Any" option was posting an empty string, notifications saved with default "Any" settings would never trigger. The validation now treats empty/null values as"-1"(the system's "Any" sentinel), allowing these notifications to process as intended.Bug Fix: NullReferenceException When No Previous State Exists
For
UnitStatusChanged,PersonnelStaffingChanged, andPersonnelStatusChangedevents, when a "before" state was required but no prior state existed (e.g., the very first state change), the code would throw a null reference exception. Null checks were added so the notification is safely skipped (returnsfalse) instead of crashing. A missing null check oncurrentStatewas also added forPersonnelStatusChanged.Bug Fix: UI Dropdown Posting Incorrect "Any" Value
The client-side dropdown initialization was changed to post
"-1"for the "Any" option instead of an empty string, aligning with the notification engine's expected value format. The API calls for populating these dropdowns were updated to stop requesting an "Any" entry from the server (includeAny=False), since it is now provided client-side.Test Coverage
New unit tests were added covering:
Summary by CodeRabbit
New Features
Bug Fixes